all major fix for the CLI done - #803
Open
Sadeequ wants to merge 3 commits into
Open
Conversation
|
@Sadeequ Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
|
@Sadeequ please fix CI |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Report: Fixes for Four CLI Issues
Issue 1: Setup default rate limits during init
I identified that the token contract's
initializefunction incontracts/token/src/lib.rswas not configuring any rate limits during initialization. The rate limiting infrastructure existed in thebc_forge_rate_limitcrate but was never invoked during the init flow.I added a
set_default_rate_limitsfunction to the token contract that configures global rate limits for all five operation types:mint,transfer,transfer_from,burn, andburn_from. Each operation type receives a default limit of 1000 operations per 60-second window. This function is called automatically at the end ofinitialize(), ensuring every newly deployed contract starts with sensible rate limiting in place.File modified:
contracts/token/src/lib.rsIssue 2: Initialize token metadata (name, symbol, decimals)
I found that the CLI's
config initcommand was generating a template.bc-forge.jsonfile without anadminfield, even though theinitializecommand required it. This created a poor user experience where the generated config was immediately insufficient for deployment.I made three changes:
Added the
--adminoption to theconfig initcommand incli/src/index.ts, allowing users to specify the admin address when creating the template config file.Added the
pauserfield to theBcForgeConfiginterface incli/src/utils/config-parser.ts, extending the config schema to support the Pauser role address.Added the
pauserfield to the JSON schema incli/src/schema/bc-forge.schema.jsonwith the Stellar G-address pattern validation (^G[A-Z2-7]{55}$).Files modified:
cli/src/index.ts,cli/src/utils/config-parser.ts,cli/src/schema/bc-forge.schema.jsonIssue 3: Link Pauser role to multisig address
I determined that the CLI had no mechanism to grant the Pauser role to a multisig address during initialization. The contract supported role-based access control with a
Pauserrole, but the CLI only exposedgrantMinterandrevokeMinteroperations.I implemented this fix by:
Adding
--pauseroption to theinitializecommand incli/src/index.ts. When provided, the CLI grants the Pauser role to the specified address immediately after successful contract initialization.Adding
grantPauser()andrevokePauser()methods to the SDK client insdk/src/client.ts. These invoke the contract'sgrant_roleandrevoke_rolefunctions with theRole::Pauserparameter.Files modified:
cli/src/index.ts,sdk/src/client.tsIssue 4: Verify on-chain state post-init
I recognized that the CLI had no way to confirm that the on-chain state matched the intended configuration after initialization. If a transaction succeeded but the contract state was unexpected (e.g., due to a simulation/execution mismatch), the user would have no indication.
I implemented post-init verification by:
Adding a
--verifyflag to theinitializecommand incli/src/index.ts. When enabled, the CLI queries the contract state after initialization and compares it against the expected values.Adding the
verifyInitializedState()method to the SDK client insdk/src/client.ts. This method queries the contract for its current admin, name, symbol, decimals, and total supply, and optionally checks whether the Pauser role was granted to the expected address. It returns anInitVerificationResultobject containing the verification status and any discrepancies.Adding
getAdmin()andhasRole()methods to the SDK client for querying contract state.Adding the
InitVerificationResultinterface tosdk/src/client.tsand exporting it fromsdk/src/index.ts.Files modified:
cli/src/index.ts,sdk/src/client.ts,sdk/src/index.tsVerification
I confirmed all changes compile successfully:
sdk/) passestsc --noEmitwith no errorscli/) passestsc --noEmitwith no errorsRelated Issues